fix(node): save install settings in lock identity by risu729 · Pull Request #9991 · jdx/mise

risu729 · 2026-05-19T12:11:20Z

Summary

save effective node.* install settings in the Node lockfile option identity
include non-empty defaults as stable lock values or markers, including mirror_url, compile = auto, make, ninja = auto, and concurrency = auto
add the default Node option identity to mise.lock while preserving the existing platform metadata
keep Node-specific [tools] options out of scope for this PR

Classification

Lock identity / stale-lock invalidation fix.

node.* settings can change either the selected Node archive or the source-build inputs for the same Node version. Those settings need to be part of the exact lockfile option key so changing configuration does not reuse a lock entry created with different install inputs.

Default Settings

Defaults are intentionally written to the lock identity when they are non-empty. This makes lockfiles stale if a future mise release changes a Node install default. Without recording defaults, an old lock entry created under one default could still match after the default changes, even though the effective install inputs are no longer the same.

Defaults that are host-dependent are recorded as stable markers instead of resolved local values. For example, unset Ninja mode is stored as ninja = auto and unset source-build concurrency is stored as concurrency = auto, so the lock identity captures the default behavior without depending on the current machine's PATH or CPU count.

Source-build-only settings are recorded when source builds can be used (compile = true or the default auto fallback path). When compile = false, source-build settings are not part of the effective install path and are omitted. Empty optional settings are also omitted to avoid meaningless lock keys.

Env Fallback

The legacy env fallback is still required for lock identity because the Node installer still honors those variables during source builds (NODE_BUILD_MIRROR_URL, NODE_CFLAGS, NODE_CONFIGURE_OPTS, NODE_MAKE_OPTS, and NODE_MAKE_INSTALL_OPTS). If lockfile option resolution ignored them, changing one of those env vars could reuse a lock entry produced for different source-build inputs.

The MISE_NODE_* env vars are covered through normal node.* settings resolution.

Verification

cargo fmt --check
git diff --check
cargo test node_lockfile_options
mise run render
cargo clippy -- -D warnings

gemini-code-assist

Code Review

This pull request introduces InstallManifest as a new source for tool options, allowing settings from the install manifest to be incorporated into the option resolution process. It also refactors the Node.js plugin to include additional build configurations, such as mirror_url and cflags, in the lockfile identity. Feedback identifies concerns regarding lockfile portability and stability in the Node.js plugin, specifically pointing out that platform-specific checks and direct environment variable access for build options can cause inconsistent lock identities across different systems.

greptile-apps · 2026-05-19T12:15:27Z

Greptile Summary

This PR expands the Node lockfile option identity to include all effective node.* install settings — mirror_url, compile, source-build inputs (cflags, configure_opts, make, make_opts, make_install_opts, ninja, concurrency), and flavor — so that changing any of those settings correctly invalidates a cached lock entry. Non-empty defaults (e.g., the default mirror URL, compile = "auto", make = "make") are intentionally recorded so future default changes also invalidate old entries.

The compile key now always appears as "true", "false", or "auto", and source-build-only settings are gated behind compile != false, matching the PR description's intent.
Legacy env-var fallbacks (NODE_BUILD_MIRROR_URL, NODE_CFLAGS, NODE_CONFIGURE_OPTS, NODE_MAKE_OPTS, NODE_MAKE_INSTALL_OPTS) are captured either via the existing accessor methods or inline env::var calls, consistent with what the installer itself reads.
The mise.lock file is updated to reflect the new identity for the project's own Node tooling.

Confidence Score: 5/5

Safe to merge — the change is a lock-identity expansion that adds fields without removing existing ones, and the new test suite validates all key branches.

The rewrite of resolve_lockfile_options is well-contained: it only produces a BTreeMap that feeds into the lock key, so the worst realistic outcome of a bug here is a stale lock reuse or spurious re-install — not data corruption or a security issue. Env-var fallbacks for all four source-build inputs are correctly captured. Default values are recorded rather than omitted, which is the right call for future-proofing. The existing unit tests cover the default path, binary-only path, and full source-build path.

No files require special attention

Important Files Changed

Filename	Overview
src/plugins/core/node.rs	Rewrites resolve_lockfile_options to capture all effective node settings in the lock identity; adds a thorough unit test suite with a Settings reset guard for isolation
mise.lock	Adds [tools.node.options] section with default values (compile=auto, concurrency=auto, make=make, mirror_url, ninja=auto) as produced by the updated resolve_lockfile_options

_{Reviews (10): Last reviewed commit: "fix(node): stabilize lock default settin..." | Re-trigger Greptile}

risu729 · 2026-05-19T12:49:47Z

CI note: lint/test-ci are failing because cargo deny check now detects RUSTSEC-2026-0145 for astral-tokio-tar 0.6.1 via rattler_package_streaming. This appears unrelated to this PR: the current main branch is failing the same cargo-deny advisory in the test workflow, and test-ci only failed because lint failed.

This comment was generated by an AI coding assistant.

coderabbitai · 2026-05-31T16:25:29Z

📝 Walkthrough

Walkthrough

resolve_lockfile_options now always builds lockfile options from settings.node and environment variables; it conditionally includes mirror_url, flavor, and compile=true, and when compile is enabled records source-build inputs (cflags, configure_opts, make, make_opts, make_install_opts, ninja). Tests add controlled Settings reset and assert these options.

Changes

Node Lockfile Options Refactoring

Layer / File(s)	Summary
Lockfile options construction `src/plugins/core/node.rs`	`resolve_lockfile_options` now derives options regardless of `target.is_current()`, conditionally inserts `mirror_url` when non-default, `compile=true` when enabled, adds `flavor` when set, and records source-build inputs (`cflags`, `configure_opts`, `make`, `make_opts`, `make_install_opts`, `ninja`) from `settings.node` and NODE_* env vars.
Test settings reset helper `src/plugins/core/node.rs`	Adds a static mutex, a drop guard to reset global `Settings`, and a helper that applies `SettingsPartial`, builds a `ToolRequest` for a current target, and calls `NodePlugin::resolve_lockfile_options` for deterministic tests.
Unit tests for lockfile options `src/plugins/core/node.rs`	Adds tests asserting `mirror_url` and `flavor` presence when configured, and that enabling `compile` yields `compile=true` plus the full set of source-build option keys with configured values.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 I hopped through Settings and code,
Options collected, neatly stowed,
Mirror, flavor, compile in sight,
Build flags gathered, tests say "right",
A tiny hop — the lockfile glows. 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	⚠️ Warning	The PR title 'fix(node): save install settings in lock identity' is partially related to the changeset but does not accurately capture the main change, which is about including source-build options and configuration in the lock identity.	Update the title to more accurately reflect the primary change, such as 'fix(node): include source build options in lock identity' or 'fix(node): include compile settings in lock identity'.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

risu729 · 2026-05-31T17:38:41Z

CI note: the current failures appear unrelated to this PR. e2e-2 fails in tasks/test_task_completion because usage complete-word now returns completion descriptions (mise.toml\tmise.toml, mise.usage.kdl\tmise.usage.kdl) where the test expects only values. e2e-3 fails the same way in tasks/test_task_completion_global_cd (alpha\talpha, beta\tbeta, gamma\tgamma).

I checked another recent non-Node PR/test run (fix(python): include sysconfig patch option in lock identity, run 26718815146) and it fails the same two e2e tranches with the same completion-output mismatch. This PR only changes Node lockfile identity handling in src/plugins/core/node.rs.

This comment was generated by an AI coding assistant.

gemini-code-assist Bot reviewed May 19, 2026

View reviewed changes

Comment thread src/plugins/core/node.rs Outdated

Comment thread src/plugins/core/node.rs Outdated

greptile-apps Bot reviewed May 19, 2026

View reviewed changes

Comment thread src/plugins/core/node.rs Outdated

greptile-apps Bot reviewed May 19, 2026

View reviewed changes

Comment thread src/plugins/core/node.rs Outdated

risu729 added 3 commits May 31, 2026 18:18

fix(node): include source build options in lock identity

81d7bf1

fix(node): keep source build lock inputs portable

bdc7c5c

fix(node): include legacy env build inputs

4a5da17

risu729 force-pushed the fix/node-build-lock-options branch from 32762fe to 4a5da17 Compare May 31, 2026 08:19

greptile-apps Bot reviewed May 31, 2026

View reviewed changes

Comment thread src/plugins/core/node.rs Outdated

risu729 marked this pull request as ready for review May 31, 2026 16:25

risu729 marked this pull request as draft May 31, 2026 16:25

refactor(node): inline lockfile option resolution

985a95a

risu729 marked this pull request as ready for review May 31, 2026 17:40

risu729 marked this pull request as draft May 31, 2026 17:41

fix(node): honor tool options in lock identity

ae3081e

risu729 mentioned this pull request May 31, 2026

docs(node): document install tool options #10186

Closed

risu729 added 2 commits June 1, 2026 08:12

fix(node): remove unused settings helpers

728b0ec

fix(node): save settings in lock identity

62b15cd

risu729 changed the title ~~fix(node): include source build options in lock identity~~ fix(node): save settings in lock identity Jun 2, 2026

greptile-apps Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread src/plugins/core/node.rs Outdated

fix(node): stabilize lock default settings

de47448

risu729 changed the title ~~fix(node): save settings in lock identity~~ fix(node): save install settings in lock identity Jun 2, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(node): save install settings in lock identity#9991

fix(node): save install settings in lock identity#9991
risu729 wants to merge 8 commits into
jdx:mainfrom
risu729:fix/node-build-lock-options

risu729 commented May 19, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented May 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

risu729 commented May 19, 2026

Uh oh!

Uh oh!

coderabbitai Bot commented May 31, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (2 warnings)

Uh oh!

risu729 commented May 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

risu729 commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Classification

Default Settings

Env Fallback

Verification

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

risu729 commented May 19, 2026

Uh oh!

Uh oh!

coderabbitai Bot commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (2 warnings)

Uh oh!

risu729 commented May 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

risu729 commented May 19, 2026 •

edited

Loading

greptile-apps Bot commented May 19, 2026 •

edited

Loading

coderabbitai Bot commented May 31, 2026 •

edited

Loading